Skip to content

reworked documentation of JSONLDSerializer.#3323

Open
WhiteGobo wants to merge 2 commits intoRDFLib:7.xfrom
WhiteGobo:DocumentationSerializerJSONLD
Open

reworked documentation of JSONLDSerializer.#3323
WhiteGobo wants to merge 2 commits intoRDFLib:7.xfrom
WhiteGobo:DocumentationSerializerJSONLD

Conversation

@WhiteGobo
Copy link
Copy Markdown
Contributor

@WhiteGobo WhiteGobo commented Nov 21, 2025

Still work in correspondence to #3307.
Added missing documentation arguments.
Working towards that all arguments are accesible with inspect.

Summary of changes

Added more documentation to module and JsonLDSerializer.serialize.
Set default of use_native_types to False.
Moved Arguments to serialize arguments, instead of extracting them from **args.
No functional change, but because i dont extract anything from **args anymore, there are some changes in the program code.
Still missing explanation to argument context.
Added docstring to PLAIN_LITERAL_TYPES.
Added crosslink to graph.serialize

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Checked that all tests and type checking passes.
  • If the change adds new features or changes the RDFLib public API:
    • Created an issue to discuss the change and get in-principle agreement.
    • Considered adding an example in ./examples.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

… shown as parameter in serialize. Added docstrings. No functional change.
)

context_data = kwargs.get("context")
use_native_types = (kwargs.get("use_native_types", False),)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to point out that use_native_types was previously a tuple(False,) whereas the new code changes it to a bool.

This alters the runtime behaviour, so it may not be strictly backwards compatible. I'm not sure whether the original tuple was intentional or an oversight.

@WhiteGobo what are your thoughts on this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry, i dont know how i missed your answer.
I'm sure the change in use_native_types from (bool,) to bool is an oversight by me. I didn't want to change any possible behaviour or data in documentation changes, unless i explicitly stated it somewhere.

Copy link
Copy Markdown
Contributor Author

@WhiteGobo WhiteGobo Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use_native_types is expected as bool in from_rdf. It seems like a bug, that it defaulted to (bool,) instead.
The only time it seems to be used is in line 418 on main, and there it result in a wrong result:

native = self.use_native_types and o.datatype in PLAIN_LITERAL_TYPES
# native can be True, when use_native_types == (False,)

The behaviour wil change if we set the default value to False. Im certain with default value on True it will behave the same, but i think the behaviour wasnt intended.

I would suggest keeping the change to bool and False, but moving the merge to 8.0 instead.
Also i wonder if a test should check for this, as no test fails on changing use_native_types

@WhiteGobo WhiteGobo changed the base branch from 7.x to main April 16, 2026 10:10
@WhiteGobo WhiteGobo changed the base branch from main to 7.x April 16, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants